Skip to content

Conversation

@alexandre-cremieux
Copy link
Contributor

@alexandre-cremieux alexandre-cremieux commented Sep 28, 2024

High level description

This contributions aims to provide a local storage solution for citations relations using MVStore. Please see #11189 for more information.

Fixes #11189

Implementation details

No new dependency added.

Caching strategy

The caching logic of the citations/references is now fully handled by h2.mvstore.MVStore. The store is responsible of the in memory caching and the offline storage logic.

This aims to avoid JabRef to access the Internet if the user already searched for citations/references related to one of its bib entry.

The cache/local store is common to all the libraries managed by the JabRef instance.

Search caching high level logic

  1. Check if the fetch is possible: if the cache does not contain anything or if the local storage TTL ran out
  2. If the fetch is possible: fetch and insert into cache (in memory and locally)
  3. Read from cache (if the in-memory cache is empty then load from local storage and update in-memory cache)

Serialization

MVStoreDAO uses the canonical representation to serialize a BibEntry and the BibtexParser to deserialize it.

Configuration

  • the search service is a singleton injected in each new Citations Relations tab using the JabRef IoC provider
  • the store path is configured to target the citations folder under JabRef's app directory

Setting translation

The the local storage TTL setting message has been traduced in:

  • DE
  • FR
  • EN
  • PL
  • IT
  • BR

Refactoring

This contributions simplifies the citations/references fetching and caching logic by introducing two layers:

  • service
  • repository

This should help to make this feature more extendable without modifying orchestration logic following open/close principle.

Screen shots

Screenshot 2025-01-26 at 22 02 16 Screenshot 2025-01-26 at 22 03 02

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@alexandre-cremieux alexandre-cremieux marked this pull request as draft September 28, 2024 20:38
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

You can check review dog's comments at the tab "Files changed" of your pull request.

@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch from 9fe8522 to cbe9e96 Compare September 28, 2024 21:15
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

You can check review dog's comments at the tab "Files changed" of your pull request.

@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch from cbe9e96 to 8231340 Compare September 28, 2024 21:51
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch from 8231340 to 33967c2 Compare September 28, 2024 22:20
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch from 33967c2 to 592d4d7 Compare September 28, 2024 22:47
@koppor koppor changed the title Fix issue 11189 part 00 refactor citation relation tab logic Fix issue #11189 part 00 refactor citation relation tab logic Sep 29, 2024
@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch 2 times, most recently from d94f4d3 to 3155242 Compare September 29, 2024 13:24
Copy link
Contributor Author

@alexandre-cremieux alexandre-cremieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add code explanations to the PR

@Siedlerchr
Copy link
Member

Please no force push if not needed. All commits will be squashed when merged

* Move repository, cache, and fetcher to logic package
* Move citations model to model/citations/semanticscholar package
* Introduce service layer
* Rename LRU cache implementation
* Add tests helpers for repository
* Move logic from repository to service
* Refactor repositories
* Update tab configuration
@alexandre-cremieux alexandre-cremieux force-pushed the fix-issue-11189-part-00-refactor-citation-relation-tab-logic branch from 3155242 to 18db75e Compare September 29, 2024 15:01
@alexandre-cremieux
Copy link
Contributor Author

Please no force push if not needed. All commits will be squashed when merged

Sorry, just re-based main branch locally.

…lation-tab-logic

# Conflicts:
#	src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general looks good. Some minor comments.

Sorry for delay. Please go ahead with everything.

@koppor koppor marked this pull request as ready for review October 10, 2024 20:29
@koppor
Copy link
Member

koppor commented Oct 10, 2024

Small other comments - IntelliJ proposed to extract a method

private static SearchCitationsRelationsService getSearchCitationsRelationsService(BibEntry cited, List<BibEntry> citationsToReturn, BibEntryRelationsRepository citationsToReturn1) {

in the tests - maybe you can also include that.

@koppor
Copy link
Member

koppor commented Oct 10, 2024

@alexandre-cremieux Please pull before you continue working on it - I merged main for you (and resolved conflicts).

@alexandre-cremieux
Copy link
Contributor Author

@alexandre-cremieux Please pull before you continue working on it - I merged main for you (and resolved conflicts).

Thanks for the review and the merge. I will resume the work on this branch and apply the changes.

@koppor
Copy link
Member

koppor commented Nov 8, 2024

@alexandre-cremieux Sorry for the merge conflicts - can you handle them? I was always happy with IntelliJ's "resolve merge conflicts" dialog. Hope, it works in this case, too.

@alexandre-cremieux
Copy link
Contributor Author

alexandre-cremieux commented Nov 8, 2024

@alexandre-cremieux Sorry for the merge conflicts - can you handle them? I was always happy with IntelliJ's "resolve merge conflicts" dialog. Hope, it works in this case, too.

Hello @koppor . Seems that we have new conflicts to resolve to be able to merge main. But I will do that when the feature will be fully developed. Was quite busy last month, I resumed the work this week. PR comments were addressed.

subhramit
subhramit previously approved these changes Jun 7, 2025
@koppor koppor enabled auto-merge June 7, 2025 19:17
subhramit
subhramit previously approved these changes Jun 7, 2025
@JabRef JabRef deleted a comment from jabref-machine Jun 7, 2025
@JabRef JabRef deleted a comment from trag-bot bot Jun 7, 2025
@koppor
Copy link
Member

koppor commented Jun 7, 2025

* increase memory again from 2 GB to something higher --> check GitHub limits somehow
* check if there are parallel test in place and tell gradle to use sequential tests

I've done both - if tests are slower on the users end, we need to work on this again.

calixtus and others added 2 commits June 7, 2025 21:38
Co-authored-by: Oliver Kopp <[email protected]>
…-00-refactor-citation-relation-tab-logic' into fork/alexandre-cremieux/fix-issue-11189-part-00-refactor-citation-relation-tab-logic
calixtus
calixtus previously approved these changes Jun 7, 2025
@calixtus calixtus disabled auto-merge June 7, 2025 19:40
Co-authored-by: Oliver Kopp <[email protected]>
@calixtus calixtus enabled auto-merge June 7, 2025 19:41
@trag-bot
Copy link

trag-bot bot commented Jun 7, 2025

@trag-bot didn't find any issues in the code! ✅✨

@jabref-machine

This comment was marked as off-topic.

@calixtus calixtus added this pull request to the merge queue Jun 7, 2025
Merged via the queue into JabRef:main with commit 3135c1a Jun 7, 2025
1 check passed
@koppor koppor mentioned this pull request Jun 7, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement caching for citation relations

7 participants